Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable AOT build of GitHub_27678 under Mono #84380

Closed
wants to merge 1 commit into from

Conversation

markples
Copy link
Member

@markples markples commented Apr 5, 2023

This is a known failure (in issues.targets), but merged test groups cause the AOT failure to fail the entire build of Regression_3. MonoAotIncompatible (and RequiresProcessIsolation) separate them and allow the test to be failed (and skipped) separately.

@markples markples added the test-bug Problem in test source code (most likely) label Apr 5, 2023
@markples markples added this to the 8.0.0 milestone Apr 5, 2023
@markples markples self-assigned this Apr 5, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 5, 2023
@ghost
Copy link

ghost commented Apr 5, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

This is a known failure (in issues.targets), but merged test groups cause the AOT failure to fail the entire build of Regression_3. MonoAotIncompatible (and RequiresProcessIsolation) separate them and allow the test to be failed (and skipped) separately.

Author: markples
Assignees: markples
Labels:

test-bug, area-CodeGen-coreclr

Milestone: 8.0.0

@markples
Copy link
Member Author

markples commented Apr 5, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@markples markples changed the title Disable AOT build of GitHub_27678 under mono Disable AOT build of GitHub_27678 under Mono Apr 5, 2023
@vargaz
Copy link
Contributor

vargaz commented Apr 5, 2023

There is a fix here:
#84385


<!-- * Assertion at
/__w/1/s/src/mono/mono/mini/memory-access.c:120, condition `size < MAX_INLINE_COPY_SIZE' not met -->
<MonoAotIncompatible>true</MonoAotIncompatible>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to mark the test as MonoAOT-incompatible? I would expect that just by using the RequiresProcessIsolation you should fix the build error as the test script will never be run (having been filtered out by the issues.targets-based exclusion list for the merged wrapper); adding the MonoAotIncompatible property seems to indicate that this test can basically never run under MonoAOT by design and I don't see why it should apply here - once the MonoAOT bug has been fixed and the issues.targets exclusion removed, the test should start running again; with this annotation it will remain buried for eternity until someone happens to notice it and / or manually audit such annotations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(side note - appears to be moot for this PR given the fix)

That's a good point. I've so used to adding RequiresProcessIsolation for existing tags like GCStressIncompatible that I followed that here - perhaps considering the MonoAOTIncompatible to be a bit of documentation. But you're right and that documentation can simply be a comment. It would probably make sense to add a comment in issues.targets to remove the RPI as well. I'll do that for future issues (or here if the PR falls through for some reason). Thanks!

@markples
Copy link
Member Author

markples commented Apr 5, 2023

Closing - will modify and reopen if needed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI test-bug Problem in test source code (most likely)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants